Skip to content

Preserve MessageHeaderAccessor binding in spring-integration TracingChannelInterceptor#19303

Open
claudobahn wants to merge 1 commit into
open-telemetry:mainfrom
claudobahn:spring-integration/tracing-channel-fix
Open

Preserve MessageHeaderAccessor binding in spring-integration TracingChannelInterceptor#19303
claudobahn wants to merge 1 commit into
open-telemetry:mainfrom
claudobahn:spring-integration/tracing-channel-fix

Conversation

@claudobahn

Copy link
Copy Markdown

Fixes #6855 (closed by the reporter once they realized they could just disable the feature to stop it from breaking their system)

Problem

With spring-integration instrumentation enabled, Spring components that recover a
message's MessageHeaderAccessor stop finding it. StompBrokerRelayMessageHandler
is one such component — it requires the accessor and throws:

No header accessor (not using the SimpMessagingTemplate?)

which breaks STOMP broker relaying. The usual workaround is
otel.instrumentation.spring-integration.enabled=false.

Root cause

preSend() rebuilds the message so it can carry the injected trace context:

return MessageBuilder.fromMessage(message)
    .copyHeaders(messageHeaderAccessor.toMessageHeaders())
    .build();

toMessageHeaders() returns plain MessageHeaders. The link between a message and
its accessor lives in MutableMessageHeaders, so rebuilding this way silently drops
it and MessageHeaderAccessor.getAccessor(message, ...) returns null downstream.

Fix

Rebuilding now distinguishes three cases:

  • No bound accessor, or an ErrorMessage — keep the existing MessageBuilder
    path. ErrorMessage does so because from 5.3 on MessageBuilder carries its
    originalMessage over into the rebuilt message and createMessage cannot.
  • Mutable headersgetMutableAccessor() returns the message's own accessor,
    so the context has already been written in place; return the message unchanged.
    Rebuilding here would seal the caller's message.
  • Otherwise — seal the accessor copy and hand over its own MessageHeaders.
    Sealing gives the new message an id and stops it sharing mutable headers with
    the accessor, while createMessage preserves the binding.

All APIs used are public at the 4.1.2 compile floor.

Tests

AbstractTracingChannelInterceptorTest in the testing module, with thin subclasses
in library and javaagent, following the existing AbstractSpringIntegrationTracingTest
pattern. Messages are sent through a real channel and asserted on what the handler
receives, over both a direct and an executor channel.

Covers context propagation, payload/header preservation, message id, header
immutability, cross-send header isolation, ErrorMessage type and originalMessage,
STOMP native headers, and accessor preservation for mutable and immutable messages.

Each of the three branches was removed in turn to confirm a test fails for each.

Verification

library / javaagent / testing check green
testLatestDeps (5.5.20), both modules green
muzzle 20 assertions — pass 4.1.0.RELEASE → 7.1.0, fail below the floor
pristine upstream against the new tests 3 failures

testLatestDeps is capped at 5.+ by a documented limitation, so muzzle is the only
coverage of 6.x/7.x. The assertInverse boundary is unmoved at 4.1.0.

Behaviour notes

Timestamp. On the sealed path the rebuilt message has no timestamp, where the
previous code generated one. setEnableTimestamp is protected so it cannot be
restored externally. This matches Spring's own idiom — accessor-bound messages built
through createMessage carry no timestamp, and the input messages in this shape don't
either — but it is a change. The message id is present; for mutable accessor-bound
messages id was already absent before this change.

Accessor subclasses. Only subclasses that override createAccessor are restored,
because getMutableAccessor delegates to it. Every Spring accessor intended to survive
a mutable copy does so; a subclass that doesn't will still resolve to the base type.

ErrorMessage. originalMessage preservation is guaranteed structurally rather than
by assertion at the baseline: neither the constructor that populates it nor
getOriginalMessage() exists at the 4.1.2 compile floor, so that test is
reflection-based and gated on testLatestDeps().

Not included

A test driving StompBrokerRelayMessageHandler end to end. Its accessor check sits
behind an isBrokerAvailable() guard, so without a live broker connection the handler
returns early and such a test would pass vacuously. The StompHeaderAccessor scenario
covers the precondition instead, including the native headers a STOMP frame actually
transmits. Happy to add a broker-backed test with guidance.

…hannelInterceptor

preSend() rebuilds the message so it can carry the injected trace context. It
did so with MessageBuilder.fromMessage(..).copyHeaders(..).build(), which
produces plain MessageHeaders. That drops the MutableMessageHeaders binding
between the message and its MessageHeaderAccessor, so components that recover
the accessor via MessageHeaderAccessor.getAccessor() no longer find it.

StompBrokerRelayMessageHandler is one such component: it requires the accessor
and throws "No header accessor (not using the SimpMessagingTemplate?)" when it
is missing, which makes STOMP broker relaying fail while this instrumentation
is enabled.

Rebuilding now distinguishes three cases. A message with no bound accessor, and
any ErrorMessage, keep the existing MessageBuilder path; ErrorMessage does so
because from 5.3 on MessageBuilder carries its originalMessage over into the
rebuilt message and createMessage cannot. When the message's headers are
mutable, getMutableAccessor() returns the message's own accessor and the
context has already been written in place, so the message is returned
unchanged; rebuilding it would seal the caller's message. Otherwise the
accessor copy is sealed and its own MessageHeaders are handed to the new
message, which preserves the binding while still giving the message an id and
preventing it from sharing mutable headers with the accessor.

Adds AbstractTracingChannelInterceptorTest covering context propagation,
payload and header preservation, message id, header immutability, ErrorMessage
type and originalMessage, STOMP native headers, and accessor preservation for
mutable and immutable messages, over both a direct and an executor channel,
from the library and javaagent test modules.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 23, 2026 02:47
@claudobahn
claudobahn requested a review from a team as a code owner July 23, 2026 02:47
@linux-foundation-easycla

Copy link
Copy Markdown

CLA Missing ID

One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via:

Co-authored-by: name <email>

Supported Co-authored-by: formats include:

  1. Anything <id+login@users.noreply.github.com> - it will locate your GitHub user by id part.
  2. Anything <login@users.noreply.github.com> - it will locate your GitHub user by login part.
  3. Anything <public-email> - it will locate your GitHub user by public-email part. Note that this email must be made public on Github.
  4. Anything <other-email> - it will locate your GitHub user by other-email part but only if that email was used before for any other CLA as a main commit author.
  5. login <any-valid-email> - it will locate your GitHub user by login part, note that login part must be at least 3 characters long.

Alternatively, if the co-author should not be included, remove the Co-authored-by: line from the commit message.

Please update your commit message(s) by doing git commit --amend and then git push [--force] and then request re-running CLA check via commenting on this pull request:

/easycla

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 23, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-23 14:31:26 UTC.

  • Waiting on: Author
  • Next step: Investigate required status check failures.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers. If the last refreshed time above predates your latest reply or push, the dashboard hasn't processed it yet.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Preserves Spring MessageHeaderAccessor bindings when injecting trace context, preventing downstream STOMP handling failures.

Changes:

  • Handles mutable, immutable, unbound, and ErrorMessage cases appropriately.
  • Adds shared tests for accessor, header, payload, context, and message preservation.
  • Runs the tests for both library and Java agent instrumentation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
testing/.../AbstractTracingChannelInterceptorTest.java Adds shared regression coverage.
library/.../TracingChannelInterceptorTest.java Enables shared library tests.
library/.../TracingChannelInterceptor.java Preserves accessor bindings during message reconstruction.
javaagent/.../TracingChannelInterceptorTest.java Enables shared Java agent tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed to handle GenericMessage. IllegalStateException: No header accessor (not using the SimpMessagingTemplate?)

2 participants